feat(common): add tag! macro #453
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
profiling
Merged
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## julio/APMSP-1004-add-metrics #453 +/- ##
================================================================
- Coverage 68.23% 68.05% -0.18%
================================================================
Files 193 192 -1
Lines 24899 24746 -153
================================================================
- Hits 16989 16841 -148
+ Misses 7910 7905 -5
|
hoolioh
force-pushed
the
julio/APMSP-1004-add-metrics
branch
2 times, most recently
from
dbb87cd to
711e6e2
Compare
danielsn
reviewed
May 28, 2024
morrisonlevi
force-pushed
the
levi/tag-macro
branch
from
b80b957 to
4e44e7e
Compare
This does a few things:
1. Creates a `tag!` macro which does compile-time checking for some
invariants.
2. Changes a `Tag` to use `Cow<'static, str>` instead of a `String`.
As libdatadog usage has grown, we have more Rust users of the lib
and therefore more static Rust strs we can trust at compile-time
and avoid runtime allocations.
3. Makes `Tag::from_value` private. Use `tag!` or `Tag::new`.
morrisonlevi
force-pushed
the
levi/tag-macro
branch
from
4e44e7e to
2c5d453
Compare
|
This is cool. While reviewing #450 I was thinking to myself "man, it sucks we can't know these tags are valid at compile time". |
danielsn
reviewed
May 29, 2024
ddcommon/src/tag.rs
Outdated
| /// the <KEY>:<VALUE> format are preferred. | ||
| pub fn from_value<'a, IntoCow>(chunk: IntoCow) -> Result<Self, Cow<'static, str>> | ||
| /// Validates a tag. | ||
| fn from_value<'a, IntoCow>(chunk: IntoCow) -> Result<Self, Cow<'static, str>> |
There was a problem hiding this comment.
That would make it a public since it's a trait impl. It's an implementation detail used by parse_tags and new.
danielsn
reviewed
May 29, 2024
bantonsson
approved these changes
May 30, 2024
ekump
approved these changes
May 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This does a few things:
tag!macro which does compile-time checking for some invariants.Tagto useCow<'static, str>instead of aString.Tag::from_valueprivate. Usetag!if the key and value are known at compile-time. If not, useTag::new.Tag::newreturns ananyhow::Result<Tag>now instead of aResult<Tag, Cow<_>>.Motivation
As libdatadog usage has grown, we have more Rust users of the lib and therefore more static Rust strs we can trust at compile-time and avoid runtime allocations.
Additional Notes
This was previously stacked on another PR, but not anymore.
How to test the change?
Everything should run as normal. It's recommended to use
tag!overTag::newif the key and value are both static strs.